Fix exit codes, error handling, and 21 TestPrograms failures - #556

scripts/run_integration_tests.sh (1)

189-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Misleading failure message for timed-out expected-fail tests.

When an expected-fail test times out (exit_code=124), it falls into the else branch and prints "FAIL $test_name (expected a nonzero exit, got $exit_code)" — but 124 is nonzero; the real problem is that it timed out rather than exiting with a genuine error code. This message could mislead someone debugging CI output into thinking the test exited with code 0.

✏️ Suggested message fix

                 if [ $exit_code -ne 0 ] && [ $exit_code -ne 124 ]; then
                     print_success "PASS $test_name (expected failure, exit code: $exit_code)"
                     passed_programs=$((passed_programs + 1))
+                elif [ $exit_code -eq 124 ]; then
+                    print_error "TIMEOUT $test_name (exceeded ${TEST_TIMEOUT}s, expected a real failure)"
+                    failed_programs=$((failed_programs + 1))
                 else
                     print_error "FAIL $test_name (expected a nonzero exit, got $exit_code)"
                     failed_programs=$((failed_programs + 1))
                 fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/run_integration_tests.sh` around lines 189 - 197, The expected-fail
handling in is_expected_fail within the integration test runner treats exit_code
124 as a generic nonzero failure, which makes the failure message misleading.
Update the branch around print_error/print_success so timed-out expected-fail
tests are reported explicitly as a timeout when exit_code is 124, while
preserving the current success path for other nonzero exits. Keep the change
localized to the expected-fail block in the shell script and adjust the message
text to reflect the actual condition.

src/builtins.rs (1)

308-340: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Document create_datetime's minimum arity
create_datetime accepts 3–6 args at runtime, but get_function_arity exposes it as 3 and that value also feeds builtin type inference. The direct-call path uses arguments.len(), so execution isn’t bounded here, but the signature is still misleading; add a short note like the timestamp entry.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/builtins.rs` around lines 308 - 340, Update get_function_arity in
src/builtins.rs to clarify that create_datetime is only documented with a
minimum arity of 3, since it can accept 3–6 arguments at runtime. Keep the
existing arity value used by builtin type inference and direct-call handling,
but add a brief inline note near the create_datetime entry, similar to the
timestamp comment, so the signature is not misleading and future readers
understand the accepted range.

TestPrograms/docs_examples/keyword_reference/contextual_keywords_examples.wfl (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Track the "needs a docs-example fix pass" TODO.

This and the other four docs_examples/keyword_reference/*.wfl files are permanently skipped with only a vague inline note, no tracked issue. Per coding guidelines, doc examples must be validated before shipping; parking them indefinitely without a tracked follow-up risks them being forgotten. Consider filing an issue per broken example (or one umbrella issue) referencing the CI-SKIP comments.

Want me to open tracking issues for these skipped docs examples?

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@TestPrograms/docs_examples/keyword_reference/contextual_keywords_examples.wfl`
at line 1, The docs example is being permanently skipped with only an untracked
inline note, which leaves the required follow-up undocumented. Replace the vague
CI-SKIP note in contextual_keywords_examples.wfl with a tracked reference to an
issue, and do the same for the other docs_examples/keyword_reference/*.wfl
files; use the existing CI-SKIP comment as the marker and add either one
umbrella issue or per-file issue references so the docs-example fix pass is
explicitly tracked.

Source: Coding guidelines

TestPrograms/web_route_params_test.wfl (1)

1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant/dead CI-SKIP comment on line 2.

The CI workflow only inspects the first line of the file for the CI-SKIP: directive. The pre-existing comment on line 2 (// CI-SKIP: starts server and waits for requests) is now dead documentation and could confuse readers into thinking it also triggers skipping.

🧹 Proposed cleanup

 // CI-SKIP: starts a web server and needs an HTTP client to drive it (covered by run_web_tests)
-// CI-SKIP: starts server and waits for requests
 // Route parameter extraction E2E test, driven by scripts/run_web_tests.sh.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@TestPrograms/web_route_params_test.wfl` around lines 1 - 2, The second
CI-SKIP comment is dead documentation because only the first line directive is
read by the workflow. Remove the redundant skip comment from
web_route_params_test.wfl and keep only the intended top-line CI-SKIP directive
so the intent is clear; this cleanup is in the test file’s header comments and
does not require changes to the test logic itself.

TestPrograms/debug_random.wfl (1)

2-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

"With parentheses" and "without parentheses" sections are now identical.

Both blocks now call random/random_boolean without parentheses, so this debug script no longer actually contrasts the two calling conventions it claims to test (labels at Lines 2 and 6, and Lines 10 and 14, describe different scenarios but exercise identical syntax).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@TestPrograms/debug_random.wfl` around lines 2 - 12, The debug script’s “with
parentheses” and “without parentheses” checks are identical, so it no longer
tests two calling conventions. Update the logic in debug_random.wfl around the
random and random_boolean calls so the sections labeled by the display
statements actually differ: one should invoke the functions with parentheses and
the other without, using the store/display statements for r1, r2, and r3 to
verify both forms.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes